[libc++] Merge the private iterator_traits aliases with their ranges versions #162661
+231
−250
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions ,cpp,h -- libcxx/include/__algorithm/copy.h libcxx/include/__algorithm/copy_backward.h libcxx/include/__algorithm/count.h libcxx/include/__algorithm/count_if.h libcxx/include/__algorithm/is_permutation.h libcxx/include/__algorithm/iterator_operations.h libcxx/include/__algorithm/lexicographical_compare_three_way.h libcxx/include/__algorithm/make_heap.h libcxx/include/__algorithm/mismatch.h libcxx/include/__algorithm/move.h libcxx/include/__algorithm/move_backward.h libcxx/include/__algorithm/pstl.h libcxx/include/__algorithm/radix_sort.h libcxx/include/__algorithm/ranges_count.h libcxx/include/__algorithm/ranges_count_if.h libcxx/include/__algorithm/ranges_unique_copy.h libcxx/include/__algorithm/sample.h libcxx/include/__algorithm/sift_down.h libcxx/include/__algorithm/stable_sort.h libcxx/include/__algorithm/unique_copy.h libcxx/include/__debug_utils/strict_weak_ordering_check.h libcxx/include/__flat_set/flat_multiset.h libcxx/include/__flat_set/flat_set.h libcxx/include/__iterator/bounded_iter.h libcxx/include/__iterator/cpp17_iterator_concepts.h libcxx/include/__iterator/iterator_traits.h libcxx/include/__iterator/static_bounded_iter.h libcxx/include/__iterator/wrap_iter.h libcxx/include/__numeric/pstl.h libcxx/include/__pstl/backends/default.h libcxx/include/__pstl/backends/libdispatch.h libcxx/include/__pstl/cpu_algos/find_if.h libcxx/include/__pstl/cpu_algos/transform.h libcxx/include/__pstl/cpu_algos/transform_reduce.h libcxx/include/__vector/vector.h libcxx/include/deque libcxx/include/forward_list libcxx/include/list libcxx/include/queue libcxx/include/set libcxx/include/stack libcxx/include/string libcxx/include/unordered_set libcxx/test/libcxx/algorithms/cpp17_iterator_concepts.verify.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/libcxx/include/__algorithm/unique_copy.h b/libcxx/include/__algorithm/unique_copy.h
index d591b8b3c..10be73021 100644
--- a/libcxx/include/__algorithm/unique_copy.h
+++ b/libcxx/include/__algorithm/unique_copy.h
@@ -109,9 +109,7 @@ unique_copy(_InputIterator __first, _InputIterator __last, _OutputIterator __res
typename iterator_traits<_OutputIterator>::value_type>::value,
__unique_copy_tags::__reread_from_output_tag,
__unique_copy_tags::__read_from_tmp_value_tag> >;
- return std::__unique_copy(
- std::move(__first), std::move(__last), std::move(__result), __pred, __algo_tag())
- .second;
+ return std::__unique_copy(std::move(__first), std::move(__last), std::move(__result), __pred, __algo_tag()).second;
}
template <class _InputIterator, class _OutputIterator>
|
philnik777
force-pushed
the
merge_iter_traits_aliases
branch
2 times, most recently
from
6d5fe98 to
44b6383
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThis simplifies the library quite a bit, since we don't need to differentiate anymore between the classic and the ranges versions of these aliases. This results in us being more forgiving in the classic algorithms, but since they're already not really enforcing the semantic requirements I don't think that's a big problem. Patch is 83.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162661.diff 44 Files Affected:
diff --git a/libcxx/include/__algorithm/copy.h b/libcxx/include/__algorithm/copy.h
index 21fd25ce6fcdc..a4344c0d711cf 100644
--- a/libcxx/include/__algorithm/copy.h
+++ b/libcxx/include/__algorithm/copy.h
@@ -197,8 +197,7 @@ struct __copy_impl {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_InIter, _OutIter>
operator()(_InIter __first, _InIter __last, _OutIter __result) const {
using _Traits = __segmented_iterator_traits<_OutIter>;
- using _DiffT =
- typename common_type<__iterator_difference_type<_InIter>, __iterator_difference_type<_OutIter> >::type;
+ using _DiffT = typename common_type<__iter_difference_t<_InIter>, __iter_difference_t<_OutIter> >::type;
if (__first == __last)
return std::make_pair(std::move(__first), std::move(__result));
diff --git a/libcxx/include/__algorithm/copy_backward.h b/libcxx/include/__algorithm/copy_backward.h
index 6c9eba672e154..48a365a1079e8 100644
--- a/libcxx/include/__algorithm/copy_backward.h
+++ b/libcxx/include/__algorithm/copy_backward.h
@@ -214,8 +214,7 @@ struct __copy_backward_impl {
auto __local_last = _Traits::__local(__result);
while (true) {
- using _DiffT =
- typename common_type<__iterator_difference_type<_InIter>, __iterator_difference_type<_OutIter> >::type;
+ using _DiffT = typename common_type<__iter_difference_t<_InIter>, __iter_difference_t<_OutIter> >::type;
auto __local_first = _Traits::__begin(__segment_iterator);
auto __size = std::min<_DiffT>(__local_last - __local_first, __last - __first);
diff --git a/libcxx/include/__algorithm/count.h b/libcxx/include/__algorithm/count.h
index 8529d110a30aa..eef77c0055340 100644
--- a/libcxx/include/__algorithm/count.h
+++ b/libcxx/include/__algorithm/count.h
@@ -10,7 +10,6 @@
#ifndef _LIBCPP___ALGORITHM_COUNT_H
#define _LIBCPP___ALGORITHM_COUNT_H
-#include <__algorithm/iterator_operations.h>
#include <__algorithm/min.h>
#include <__bit/invert_if.h>
#include <__bit/popcount.h>
@@ -31,10 +30,10 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
// generic implementation
-template <class _AlgPolicy, class _Iter, class _Sent, class _Tp, class _Proj>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 typename _IterOps<_AlgPolicy>::template __difference_type<_Iter>
+template <class _Iter, class _Sent, class _Tp, class _Proj>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iter_difference_t<_Iter>
__count(_Iter __first, _Sent __last, const _Tp& __value, _Proj& __proj) {
- typename _IterOps<_AlgPolicy>::template __difference_type<_Iter> __r(0);
+ __iter_difference_t<_Iter> __r(0);
for (; __first != __last; ++__first)
if (std::__invoke(__proj, *__first) == __value)
++__r;
@@ -71,8 +70,8 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_t
return __r;
}
-template <class, class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iterator_difference_type<__bit_iterator<_Cp, _IsConst> >
+template <class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iter_difference_t<__bit_iterator<_Cp, _IsConst> >
__count(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, const _Tp& __value, _Proj&) {
if (__value)
return std::__count_bool<true>(
@@ -82,10 +81,10 @@ __count(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __l
}
template <class _InputIterator, class _Tp>
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iterator_difference_type<_InputIterator>
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iter_difference_t<_InputIterator>
count(_InputIterator __first, _InputIterator __last, const _Tp& __value) {
__identity __proj;
- return std::__count<_ClassicAlgPolicy>(__first, __last, __value, __proj);
+ return std::__count(__first, __last, __value, __proj);
}
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__algorithm/count_if.h b/libcxx/include/__algorithm/count_if.h
index 26f945e6bd98c..46fa2e64c9b74 100644
--- a/libcxx/include/__algorithm/count_if.h
+++ b/libcxx/include/__algorithm/count_if.h
@@ -10,7 +10,6 @@
#ifndef _LIBCPP___ALGORITHM_COUNT_IF_H
#define _LIBCPP___ALGORITHM_COUNT_IF_H
-#include <__algorithm/iterator_operations.h>
#include <__config>
#include <__functional/identity.h>
#include <__iterator/iterator_traits.h>
@@ -22,10 +21,10 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _AlgPolicy, class _Iter, class _Sent, class _Proj, class _Pred>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __policy_iter_diff_t<_AlgPolicy, _Iter>
+template <class _Iter, class _Sent, class _Proj, class _Pred>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __iter_difference_t<_Iter>
__count_if(_Iter __first, _Sent __last, _Pred& __pred, _Proj& __proj) {
- __policy_iter_diff_t<_AlgPolicy, _Iter> __counter(0);
+ __iter_difference_t<_Iter> __counter(0);
for (; __first != __last; ++__first) {
if (std::__invoke(__pred, std::__invoke(__proj, *__first)))
++__counter;
@@ -38,7 +37,7 @@ template <class _InputIterator, class _Predicate>
typename iterator_traits<_InputIterator>::difference_type
count_if(_InputIterator __first, _InputIterator __last, _Predicate __pred) {
__identity __proj;
- return std::__count_if<_ClassicAlgPolicy>(__first, __last, __pred, __proj);
+ return std::__count_if(__first, __last, __pred, __proj);
}
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__algorithm/is_permutation.h b/libcxx/include/__algorithm/is_permutation.h
index 86f469c2799c5..0908001cd1a24 100644
--- a/libcxx/include/__algorithm/is_permutation.h
+++ b/libcxx/include/__algorithm/is_permutation.h
@@ -78,7 +78,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __is_permutation_impl(
_Pred&& __pred,
_Proj1&& __proj1,
_Proj2&& __proj2) {
- using _D1 = __iterator_difference_type<_Iter1>;
+ using _D1 = __iter_difference_t<_Iter1>;
for (auto __i = __first1; __i != __last1; ++__i) {
// Have we already counted the number of *__i in [f1, l1)?
@@ -126,7 +126,7 @@ template <class _AlgPolicy, class _ForwardIterator1, class _Sentinel1, class _Fo
return true;
// __first1 != __last1 && *__first1 != *__first2
- using _D1 = __iterator_difference_type<_ForwardIterator1>;
+ using _D1 = __iter_difference_t<_ForwardIterator1>;
_D1 __l1 = _IterOps<_AlgPolicy>::distance(__first1, __last1);
if (__l1 == _D1(1))
return false;
@@ -173,10 +173,10 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __is_permutation(
if (__first2 == __last2) // Second range is shorter
return false;
- using _D1 = __iterator_difference_type<_Iter1>;
+ using _D1 = __iter_difference_t<_Iter1>;
_D1 __l1 = _IterOps<_AlgPolicy>::distance(__first1, __last1);
- using _D2 = __iterator_difference_type<_Iter2>;
+ using _D2 = __iter_difference_t<_Iter2>;
_D2 __l2 = _IterOps<_AlgPolicy>::distance(__first2, __last2);
if (__l1 != __l2)
return false;
diff --git a/libcxx/include/__algorithm/iterator_operations.h b/libcxx/include/__algorithm/iterator_operations.h
index e5c89c1e67e3a..375ac1db1a1a8 100644
--- a/libcxx/include/__algorithm/iterator_operations.h
+++ b/libcxx/include/__algorithm/iterator_operations.h
@@ -47,15 +47,9 @@ struct _RangeAlgPolicy {};
template <>
struct _IterOps<_RangeAlgPolicy> {
- template <class _Iter>
- using __value_type _LIBCPP_NODEBUG = iter_value_t<_Iter>;
-
template <class _Iter>
using __iterator_category _LIBCPP_NODEBUG = ranges::__iterator_concept<_Iter>;
- template <class _Iter>
- using __difference_type _LIBCPP_NODEBUG = iter_difference_t<_Iter>;
-
static constexpr auto advance = ranges::advance;
static constexpr auto distance = ranges::distance;
static constexpr auto __iter_move = ranges::iter_move;
@@ -71,15 +65,9 @@ struct _ClassicAlgPolicy {};
template <>
struct _IterOps<_ClassicAlgPolicy> {
- template <class _Iter>
- using __value_type _LIBCPP_NODEBUG = typename iterator_traits<_Iter>::value_type;
-
template <class _Iter>
using __iterator_category _LIBCPP_NODEBUG = typename iterator_traits<_Iter>::iterator_category;
- template <class _Iter>
- using __difference_type _LIBCPP_NODEBUG = typename iterator_traits<_Iter>::difference_type;
-
// advance
template <class _Iter, class _Distance>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static void advance(_Iter& __iter, _Distance __count) {
@@ -164,17 +152,17 @@ struct _IterOps<_ClassicAlgPolicy> {
// advance with sentinel, a la std::ranges::advance
template <class _Iter>
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __difference_type<_Iter>
- __advance_to(_Iter& __iter, __difference_type<_Iter> __count, const _Iter& __sentinel) {
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __iter_difference_t<_Iter>
+ __advance_to(_Iter& __iter, __iter_difference_t<_Iter> __count, const _Iter& __sentinel) {
return _IterOps::__advance_to(__iter, __count, __sentinel, typename iterator_traits<_Iter>::iterator_category());
}
private:
// advance with sentinel, a la std::ranges::advance -- InputIterator specialization
template <class _InputIter>
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __difference_type<_InputIter> __advance_to(
- _InputIter& __iter, __difference_type<_InputIter> __count, const _InputIter& __sentinel, input_iterator_tag) {
- __difference_type<_InputIter> __dist = 0;
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __iter_difference_t<_InputIter> __advance_to(
+ _InputIter& __iter, __iter_difference_t<_InputIter> __count, const _InputIter& __sentinel, input_iterator_tag) {
+ __iter_difference_t<_InputIter> __dist = 0;
for (; __dist < __count && __iter != __sentinel; ++__dist)
++__iter;
return __count - __dist;
@@ -182,12 +170,12 @@ struct _IterOps<_ClassicAlgPolicy> {
// advance with sentinel, a la std::ranges::advance -- BidirectionalIterator specialization
template <class _BiDirIter>
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __difference_type<_BiDirIter>
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __iter_difference_t<_BiDirIter>
__advance_to(_BiDirIter& __iter,
- __difference_type<_BiDirIter> __count,
+ __iter_difference_t<_BiDirIter> __count,
const _BiDirIter& __sentinel,
bidirectional_iterator_tag) {
- __difference_type<_BiDirIter> __dist = 0;
+ __iter_difference_t<_BiDirIter> __dist = 0;
if (__count >= 0)
for (; __dist < __count && __iter != __sentinel; ++__dist)
++__iter;
@@ -199,9 +187,9 @@ struct _IterOps<_ClassicAlgPolicy> {
// advance with sentinel, a la std::ranges::advance -- RandomIterator specialization
template <class _RandIter>
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __difference_type<_RandIter>
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static __iter_difference_t<_RandIter>
__advance_to(_RandIter& __iter,
- __difference_type<_RandIter> __count,
+ __iter_difference_t<_RandIter> __count,
const _RandIter& __sentinel,
random_access_iterator_tag) {
auto __dist = _IterOps::distance(__iter, __sentinel);
@@ -216,9 +204,6 @@ struct _IterOps<_ClassicAlgPolicy> {
}
};
-template <class _AlgPolicy, class _Iter>
-using __policy_iter_diff_t _LIBCPP_NODEBUG = typename _IterOps<_AlgPolicy>::template __difference_type<_Iter>;
-
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
diff --git a/libcxx/include/__algorithm/lexicographical_compare_three_way.h b/libcxx/include/__algorithm/lexicographical_compare_three_way.h
index 442223e79e4ec..a350232af17b2 100644
--- a/libcxx/include/__algorithm/lexicographical_compare_three_way.h
+++ b/libcxx/include/__algorithm/lexicographical_compare_three_way.h
@@ -37,13 +37,13 @@ template <class _InputIterator1, class _InputIterator2, class _Cmp>
_LIBCPP_HIDE_FROM_ABI constexpr auto __lexicographical_compare_three_way_fast_path(
_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2, _InputIterator2 __last2, _Cmp& __comp)
-> decltype(__comp(*__first1, *__first2)) {
- static_assert(signed_integral<__iterator_difference_type<_InputIterator1>>,
+ static_assert(signed_integral<__iter_difference_t<_InputIterator1>>,
"Using a non-integral difference_type is undefined behavior.");
- static_assert(signed_integral<__iterator_difference_type<_InputIterator2>>,
+ static_assert(signed_integral<__iter_difference_t<_InputIterator2>>,
"Using a non-integral difference_type is undefined behavior.");
- using _Len1 = __iterator_difference_type<_InputIterator1>;
- using _Len2 = __iterator_difference_type<_InputIterator2>;
+ using _Len1 = __iter_difference_t<_InputIterator1>;
+ using _Len2 = __iter_difference_t<_InputIterator2>;
using _Common = common_type_t<_Len1, _Len2>;
_Len1 __len1 = __last1 - __first1;
diff --git a/libcxx/include/__algorithm/make_heap.h b/libcxx/include/__algorithm/make_heap.h
index f98a0d2f89c85..0f98d6fd80ae6 100644
--- a/libcxx/include/__algorithm/make_heap.h
+++ b/libcxx/include/__algorithm/make_heap.h
@@ -33,10 +33,10 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
__make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare&& __comp) {
__comp_ref_type<_Compare> __comp_ref = __comp;
- using __diff_t = __iterator_difference_type<_RandomAccessIterator>;
+ using __diff_t = __iter_difference_t<_RandomAccessIterator>;
const __diff_t __n = __last - __first;
- const bool __assume_both_children = is_arithmetic<__iterator_value_type<_RandomAccessIterator> >::value;
+ const bool __assume_both_children = is_arithmetic<__iter_value_t<_RandomAccessIterator> >::value;
// While it would be correct to always assume we have both children, in practice we observed this to be a performance
// improvement only for arithmetic types.
diff --git a/libcxx/include/__algorithm/mismatch.h b/libcxx/include/__algorithm/mismatch.h
index 749c701974f07..fd1e989467d2e 100644
--- a/libcxx/include/__algorithm/mismatch.h
+++ b/libcxx/include/__algorithm/mismatch.h
@@ -60,7 +60,7 @@ __mismatch(_Iter1 __first1, _Sent1 __last1, _Iter2 __first2, _Pred& __pred, _Pro
template <class _Iter>
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_Iter, _Iter>
__mismatch_vectorized(_Iter __first1, _Iter __last1, _Iter __first2) {
- using __value_type = __iterator_value_type<_Iter>;
+ using __value_type = __iter_value_t<_Iter>;
constexpr size_t __unroll_count = 4;
constexpr size_t __vec_size = __native_vector_size<__value_type>;
using __vec = __simd_vector<__value_type, __vec_size>;
diff --git a/libcxx/include/__algorithm/move.h b/libcxx/include/__algorithm/move.h
index 52bd5fb5253db..031560c3b1b76 100644
--- a/libcxx/include/__algorithm/move.h
+++ b/libcxx/include/__algorithm/move.h
@@ -80,8 +80,7 @@ struct __move_impl {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_InIter, _OutIter>
operator()(_InIter __first, _InIter __last, _OutIter __result) const {
using _Traits = __segmented_iterator_traits<_OutIter>;
- using _DiffT =
- typename common_type<__iterator_difference_type<_InIter>, __iterator_difference_type<_OutIter> >::type;
+ using _DiffT = typename common_type<__iter_difference_t<_InIter>, __iter_difference_t<_OutIter> >::type;
if (__first == __last)
return std::make_pair(std::move(__first), std::move(__result));
diff --git a/libcxx/include/__algorithm/move_backward.h b/libcxx/include/__algorithm/move_backward.h
index a4698327b474d..a6a7130b47719 100644
--- a/libcxx/include/__algorithm/move_backward.h
+++ b/libcxx/include/__algorithm/move_backward.h
@@ -86,8 +86,7 @@ struct __move_backward_impl {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_InIter, _OutIter>
operator()(_InIter __first, _InIter __last, _OutIter __result) const {
using _Traits = __segmented_iterator_traits<_OutIter>;
- using _DiffT =
- typename common_type<__iterator_difference_type<_InIter>, __iterator_difference_type<_OutIter> >::type;
+ using _DiffT = typename common_type<__iter_difference_t<_InIter>, __iter_difference_t<_OutIter> >::type;
// When the range contains no elements, __result might not be a valid iterator
if (__first == __last)
diff --git a/libcxx/include/__algorithm/pstl.h b/libcxx/include/__algorithm/pstl.h
index eea07e2b96b64..e0f4e9dac62ea 100644
--- a/libcxx/include/__algorithm/pstl.h
+++ b/libcxx/include/__algorithm/pstl.h
@@ -115,7 +115,7 @@ template <class _ExecutionPolicy,
class _Predicate,
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI __iterator_difference_type<_ForwardIterator>
+_LIBCPP_HIDE_FROM_ABI __iter_difference_t<_ForwardIterator>
count_if(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, _Predicate __pred) {
_LIBCPP_REQUIRE_CPP17_FORWARD_ITERATOR(
_ForwardIterator, "count_if(first, last, pred) requires [first, last) to be ForwardIterators");
@@ -129,7 +129,7 @@ template <class _ExecutionPolicy,
class _Tp,
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI __iterator_difference_type<_ForwardIterator>
+_LIBCPP_HIDE_FROM_ABI __iter_difference_t<_ForwardIterator>
count(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
_LIBCPP_REQUIRE_CPP17_FORWARD_ITERATOR(
_ForwardIterator, "count(first, last, val) requires [first, last) to be ForwardIterators");
diff --git a/libcxx/include/__algorithm/radix_sort.h b/libcxx/include/__algorithm/radix_sort.h
index 5549a69f5e220..290a36ca3cca0 100644
--- a/libcxx/include/__algorithm/radix_sort.h
+++ b/libcxx/include/__algorithm/radix_sort.h
@@ -72,14 +72,14 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#if _LIBCPP_STD_VER >= 14
template <class _InputIterator, class _OutputIterator>
-_LIBCPP_HIDE_FROM_ABI constexpr pair<_OutputIterator, __iterator_value_type<_InputIterator>>
+_LIBCPP_HIDE_FROM_ABI constexpr pair<_OutputIterator, __iter_value_t<_InputIterator>>
__partial_sum_max(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
if (__first == __last)
return {__result, 0};
- auto __max = *__first;
- __iterator_value_type<_InputIterator> __sum = *__first;
- *__result = __sum;
+ auto __max = *__first;
+ __iter_value_t<_InputIterator> __sum = *__first;
+ *__result = __sum;
while (++__first != __last) {
if (__max < *__first) {
@@ -124,7 +124,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr auto __nth_radix(size_t __radix_number, _Radix _
template <class _ForwardIterator, class _Map, class _RandomAccessIterator>
_LIBCPP_HIDE_FROM_ABI constexpr void
__collect(_ForwardIterator __first, _ForwardIterator __last, _Map __map, _RandomAccessIterator __counters) {
- using __value_type = __iterator_value_type<_ForwardIterator>;
+ using __value_type = __iter_value_t<_ForwardIterator>;
using __traits = __counting_sort_traits<__value_type, _Map>;
std::for_each(__first, __last, [&__counters, &__map](const auto& __preimage) { ++__counters[__map(__preimage)]; });
@@ -160,7 +160,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool __collect_impl(
_RandomAccessIterator1 __counters,
_RandomAccessIterator2 __maximums,
index_sequence<_Radices...>) {
- using __value_type = __iterator_value_type<_ForwardIterator>;
+ using __value_type = __iter_value_t<_ForwardIterator>;
constexpr auto __radix_value_range = __radix_sort_traits<__value_type, _Map, _Radix>::__radix_value_range;...
[truncated]
|
ldionne
requested changes
Oct 15, 2025
| #if _LIBCPP_STD_VER >= 23 | ||
| template <class _InputIterator> | ||
| using __iter_key_type _LIBCPP_NODEBUG = remove_const_t<tuple_element_t<0, __iterator_value_type<_InputIterator>>>; | ||
| using __iter_key_type _LIBCPP_NODEBUG = remove_const_t<tuple_element_t<0, __iter_value_t<_InputIterator>>>; |
There was a problem hiding this comment.
We were using this in the deduction guides for e.g. flat_map:
llvm-project/libcxx/include/__flat_map/flat_map.h
Line 1198 in aa43577
Were we conforming prior to this change since we were always using iterator_traits? It would be surprising.
I imagine that either there's a LWG issue because the spec is using iterator_traits for flat_map when it should use something more recent, or we used to have a slight conformance bug. If the latter, let's add test coverage for that.
There was a problem hiding this comment.
We were conforming. The constraints are introduced by the exact same wording across the map types AFAICT. It also seems intentional to me, since the rest of the wording is rather close to the legacy containers.
libcxx/test/libcxx/algorithms/cpp17_iterator_concepts.verify.cpp
Outdated
Show resolved
Hide resolved
| template <> | ||
| struct _IterOps<_ClassicAlgPolicy> { | ||
| template <class _Iter> | ||
| using __value_type _LIBCPP_NODEBUG = typename iterator_traits<_Iter>::value_type; |
There was a problem hiding this comment.
Why is it okay to make this (general) change? I understand that we are probably just accepting more code than previously, but it is possible for conforming C++20 code to break when we do this?
A few questions:
- In C++20, is it possible for an iterator to have different types for
std::iter_value_t<It>andstd::iterator_traits<It>::value_type? - If so, is it possible for these two types to differ to the extent that a classic algorithm (which should in principle use
iterator_traits, or at least did historically) would break if we start usingstd::iter_value_t<It>instead? - If it's possible for these two typedefs to differ and to the extent of code breaking, are these cases so far fetched that we don't care to support them?
- The same set of questions holds for every nested typedef we're modifying here.
Also CC @frederick-vs-ja since I think I remember having that discussion in a previous patch.
There was a problem hiding this comment.
Your proofs:
-
For
iter_reference_t, https://eel.is/c++draft/iterator.cpp17#input.iterators-2 says that*amust beiterator_traits::referencefor a C++17 iterator. Henceiterator_traits::referenceanddecltype(*a) == iter_reference_t<It>must be the same. ✅ -
For
iter_difference_t, we have two cases. Ifiterator_traits<It>is specialized, incrementable.traits says thatiter_difference_t<It>isiterator_traits<It>::difference_type. If not specialized, theniter_difference_t<It>isincrementable_traits<It>::difference_type, whatever that is. However, in C++20,iterator_traitsalso changed to be more C++20 friendly (I guess):-
if
It::difference_typeandIt::value_typeandIt::reference(etc..) do exist, theniterator_traits<It>::difference_typeisIt::difference_type. However, in that case,incrementable_traits<It>::difference_typeis alsoIt::difference_type, unless it is specialized explicitly. And it's legal to specialize it explicitly. This means that technically, if someone specializesincrementable_traits<It>to give it adifference_typethat doesn't matchiterator_traits<It>::difference_type, this patch will produce a difference. It's unclear whetherUsers may specialize incrementable_traits on program-defined types.
is intended to override the normal requirement that specializations must be equivalent to the base template. This is probably material for a quick LWG issue. ❌
-
if
It::difference_typeor any of its friends do not exist,iterator_traits<It>::difference_typeis defined to beincrementable_traits<It>::difference_type, so the equivalence holds. ✅
-
-
For
iter_value_t,iter_value_t<It>isiterator_traits<It>::value_typeifiterator_traits<It>is specialized. Otherwise, it isindirectly_readable_traits<It>::value_type, whatever that is. However,- if
It::difference_typeandIt::value_typeandIt::reference(etc..) do exist, theniterator_traits<It>::value_typeisI::value_type. And in that caseindirectly_readable_traits<It>::value_typeis a rather complicated thing. It seems that ifIt::element_typeandIt::value_typedisagree,indirectly_readable_traits<It>::value_typewould not be defined whereasiterator_traits<It>::value_typewould be. There's also the specialization problem above. And there's the issue of legacy output iterators whereIt::value_type == void, as explained in this note. ❌ - if
It::value_typeor any of its friends do not exist,iterator_traits<It>::value_typeis defined toindirectly_readable_traits<It>::value_type. ✅
- if
So, in summary, I think we have the following issues:
iterator_traits<It>::difference_typemay beIt::difference_typebutincrementable_traits<It>may be something else via a specializationiterator_traits<It>::value_typemay beIt::value_typebutindirectly_readable_traits<It>::value_typedoes not existiterator_traits<It>::value_typemay beIt::value_typebutindirectly_readable_traits<It>may be specializediterator_traits<It>::value_typemay beIt::value_type == voidbutindirectly_readable_traits<It>does not exist
There was a problem hiding this comment.
Now, the question is whether we actually care about these differences.
I think we don't care about the specialization stuff. We should get those clarified with a LWG issue, but I have no sympathy for someone who has different It::difference_type and incrementable_traits<It>::difference_type through a specialization. Same for It::value_type.
However, the case of legacy output iterators could be a real problem. However, we basically never use iterator_traits<It>::value_type for an output iterator since it may be void. So in practice this might not matter.
I'll wait for @frederick-vs-ja to chime in, but so far I think I'd be comfortable with making this change. However, this would have to be dry-run on large code bases (we can ping the Google folks and we'll also do an internal build when we're ready), to shake off any unintended consequences.
There was a problem hiding this comment.
I submitted LWG4080. Perhaps we should add some discussions to it.
It's unclear whether
Users may specialize incrementable_traits on program-defined types.
is intended to override the normal requirement that specializations must be equivalent to the base template.
I think overriding was intended. The standard library contains such design for common_iterator, although this looks quite questionable to me. If overriding is considered harmful (as it can create inconsistent value types), I think it makes more sense to disallow some user-provided specializations (and slightly fix common_iterator).
iterator_traits<It>::value_typemay beIt::value_typebutindirectly_readable_traits<It>::value_typedoes not exist
I'm more worried about this. There can be C++17 iterators with evil element_type (I initially reported this in microsoft/STL#4436) which make them not C++20 iterators. If we use __iter_value_t like MSVC STL's current approach, some code using legacy algorithms can be accepted in C++17 but rejected in C++20.
There was a problem hiding this comment.
I'm really not that worried about a value_type/element_type mismatch. That rather seems to me like something worth diagnosing, since it's almost certainly user error. I'm not sure it's actually specified anywhere, but the intention that remove_reference_t<decltype(*iter)> is element_type seems rather clear to me.
philnik777
force-pushed
the
merge_iter_traits_aliases
branch
from
44b6383 to
435b204
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This simplifies the library quite a bit, since we don't need to differentiate anymore between the classic and the ranges versions of these aliases. This results in us being more forgiving in the classic algorithms, but since they're already not really enforcing the semantic requirements I don't think that's a big problem.